Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add systemd support for Debian Jessie, Stretch, and Buster #1419

Merged
merged 1 commit into from
Feb 27, 2019

Conversation

jeking3
Copy link
Contributor

@jeking3 jeking3 commented Dec 10, 2018

Description

Adds support for waagent under Debian Jessie, Stretch, and Buster which all use systemd.


PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made and Travis.CI is passing.

Quality of Code and Contribution Guidelines


This change is Reviewable

@jeking3 jeking3 changed the title Add systemd support for Debian Buster Add systemd support for Debian Jessie, Stretch, and Buster Dec 11, 2018
@msftclas
Copy link

msftclas commented Dec 11, 2018

CLA assistant check
All CLA requirements met.

setup.py Outdated
@@ -158,6 +160,11 @@ def get_data_files(name, version, fullname):
set_conf_files(data_files, src=["config/debian/waagent.conf"])
set_logrotate_files(data_files)
set_udev_files(data_files, dest="/lib/udev/rules.d")
# All Debian versions since Jessie use systemd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this means we will update for each new version of debian? Can't you just specify the old supported version and say the other ones use systemd ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to search for a package and see if it is installed... so I agree what's here is not currently optimal however it is an improvement over what was there before, which was no systemd support at all, and the agent wasn't starting (at least on buster, for me...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some additional information on platforms, just tucking it away somewhere so I can use it later. Apparently it isn't so easy to identify debian releases with platform.dist() - of course it's been deprecated since python 2.6, but I have addressed that in another PR.

debian:jessie
    >>> print(platform.dist())
    ('debian', '8.11', '')

debian:stretch
    >>> print(platform.dist())
    ('debian', '9.7', '')

debian:buster
    >>> print(platform.dist())
    ('debian', 'buster/sid', '')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plessisa please review the change, I removed the named comparison

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@vrdmr vrdmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -177,6 +181,14 @@ def get_data_files(name, version, fullname):
return data_files


def debian_has_systemd():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of debian_has_systemd() is cleaner than the earlier approach. Thanks for doing the change. 👍

@vrdmr
Copy link
Member

vrdmr commented Feb 20, 2019

Running Automation now.

@vrdmr
Copy link
Member

vrdmr commented Feb 22, 2019

@jeking3 Dont know why TravisCI is stuck at the waiting status state. I kicked off a new build in TravisCI but it still did not report here. Does this need a new a new dummy commit?

- cleaned up some minor flake8 warnings
@jeking3
Copy link
Contributor Author

jeking3 commented Feb 22, 2019

All set. I rebased on master.

@vrdmr
Copy link
Member

vrdmr commented Feb 22, 2019

Running Automation one final time and I'll merge it when it passes. Thanks.

@narrieta narrieta changed the base branch from master to develop February 27, 2019 00:23
@vrdmr vrdmr merged commit 8aa5e4c into Azure:develop Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants